Skip to content

Comments

Westlad/identify user#1413

Open
Westlad wants to merge 11 commits intomasterfrom
westlad/identify-user
Open

Westlad/identify user#1413
Westlad wants to merge 11 commits intomasterfrom
westlad/identify-user

Conversation

@Westlad
Copy link

@Westlad Westlad commented Mar 27, 2023

What does this implement/fix? Explain your changes.

This code requires a User to prove to a Proposer that they hold a whitelisted Ethereum account. It is useful for cases where a Proposer wishes to show compliance with excluding non-whitelisted accounts. It is not strictly necessary because a non-whitelisted account cannot move funds into or out of Nightfall and therefore any transactions they may make within Nightfall are ultimately futile. However, this check provides extra assurance. It is entirely voluntary because there is no on-chain enforcement of the check. Proposers may disable the check, at their own risk, by setting the environment variable ANONYMOUS_USER.

What commands can I run to test the change?

This can be tested by running the normal test suite, whereby the check is enabled by default because ANONYMOUS_USER is not set.

@Westlad Westlad force-pushed the westlad/identify-user branch from 78e6c35 to b0f09d2 Compare March 30, 2023 14:39
@Westlad Westlad marked this pull request as ready for review March 31, 2023 15:34
Copy link
Contributor

@rael-b rael-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is code from the PR in this one. It would be good to add some sort of documentation about the variable ANONYMOUS_USER.

@Westlad
Copy link
Author

Westlad commented Apr 11, 2023

I'll add documentation of the anonymous user. This code is a branch off the PR you've mentioned, but I wanted to get that one in first, before merging this, because the other PR is a refactor whereas this adds new functionality.

@Westlad Westlad force-pushed the westlad/identify-user branch from b8fce80 to 60bc907 Compare April 12, 2023 10:04
@rael-b rael-b added the One more approval needed One reviewer has approved this PR but another is needed label Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

One more approval needed One reviewer has approved this PR but another is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants